Validation for using the same service-name for different resources#1673
Merged
vkalapov merged 4 commits intoAug 1, 2025
Merged
Conversation
ikasarov
reviewed
Jul 29, 2025
| public static final String SERVICE_INSTANCE_0_PLAN_UPDATE_FAILED_IGNORING_FAILURE = "Service instance: \"{0}\" plan update failed, ignoring failure..."; | ||
| public static final String SERVICE_INSTANCE_0_PARAMETERS_UPDATE_FAILED_IGNORING_FAILURE = "Service instance: \"{0}\" parameters update failed, ignoring failure..."; | ||
| public static final String SERVICE_INSTANCE_0_TAGS_UPDATE_FAILED_IGNORING_FAILURE = "Service instance: \"{0}\" tags update failed, ignoring failure..."; | ||
| public static final String ONLY_FIRST_SERVICE_WILL_BE_CREATED = "[WARNING] Only the first service will be created because provided 'service-name' fields are duplicated! All other services with the same 'service-name' will be ignored!"; |
Contributor
There was a problem hiding this comment.
is the '[WARNING]' text consistent with any other message?
Contributor
Author
There was a problem hiding this comment.
It isn't actually - just made more sense to me - but will make it like the others, fixed
| .collect(Collectors.toSet()); | ||
| if (resolvedServiceInstancesNames.size() != resolvedServiceInstancesNamesWithoutDuplicates.size()) { | ||
| getStepLogger().warn( | ||
| Messages.ONLY_FIRST_SERVICE_WILL_BE_CREATED); |
Contributor
There was a problem hiding this comment.
it makes more to sense to actually print out the specific duplicated service names instead of the generic warning
| context.setVariable(Variables.SERVICES_TO_CREATE_COUNT, servicesToCreate.size()); | ||
| } | ||
|
|
||
| private void checkForDuplicatedServiceNameFields(List<CloudServiceInstanceExtended> resolvedServiceInstances) { |
Contributor
There was a problem hiding this comment.
this is only in the 'batchedServices...' step - is there an alternative flow in the diagram that doesn't split the service in batches and will it not validate in that case?
ikasarov
previously approved these changes
Jul 29, 2025
karrgov
force-pushed
the
service_name_duplicating_warning
branch
from
c1a1d01 to
9e8348b
Compare
LMCROSSITXSADEPLOY-1661
karrgov
force-pushed
the
service_name_duplicating_warning
branch
3 times, most recently
from
ffec30f to
289b9c2
Compare
vkalapov
reviewed
Jul 29, 2025
| public static final String SERVICE_INSTANCE_0_PLAN_UPDATE_FAILED_IGNORING_FAILURE = "Service instance: \"{0}\" plan update failed, ignoring failure..."; | ||
| public static final String SERVICE_INSTANCE_0_PARAMETERS_UPDATE_FAILED_IGNORING_FAILURE = "Service instance: \"{0}\" parameters update failed, ignoring failure..."; | ||
| public static final String SERVICE_INSTANCE_0_TAGS_UPDATE_FAILED_IGNORING_FAILURE = "Service instance: \"{0}\" tags update failed, ignoring failure..."; | ||
| public static final String ONLY_FIRST_SERVICE_WILL_BE_CREATED = "Only the first service will be created because provided 'service-name' fields are duplicated! All other services with the same 'service-name' will be ignored! Duplicated names: "; |
Contributor
There was a problem hiding this comment.
Small typo:
".. created because the provided 'service-name' .."
| List<String> duplicatedNames = getDuplicatedNames(resolvedServiceInstancesNames); | ||
| if (!duplicatedNames.isEmpty()) { | ||
| getStepLogger().warn( | ||
| Messages.ONLY_FIRST_SERVICE_WILL_BE_CREATED + String.join(" ", duplicatedNames)); |
Contributor
There was a problem hiding this comment.
Avoid using the string concatenation with the + operator, for the message, and instead use placeholders {0}.
vkalapov
reviewed
Jul 29, 2025
| List<String> duplicatedNames = new ArrayList<>(); | ||
| Map<String, Integer> frequencyOfNamesMap = new HashMap<>(); | ||
| for (String name : resolvedServiceInstancesNames) { | ||
| frequencyOfNamesMap.put(name, frequencyOfNamesMap.getOrDefault(name, 0) + 1); |
Contributor
There was a problem hiding this comment.
This line could be replaced by frequencyOfNamesMap.merge(name, 1, Integer::sum);
Contributor
Author
There was a problem hiding this comment.
Fixed, thank you for the suggestion!
LMCROSSITXSADEPLOY-1661
This reverts commit 3ddb37b. LMCROSSITXSADEPLOY-1661
karrgov
force-pushed
the
service_name_duplicating_warning
branch
from
56242a9 to
5a045a6
Compare
LMCROSSITXSADEPLOY-1661
ikasarov
approved these changes
Jul 30, 2025
vkalapov
approved these changes
Jul 30, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
LMCROSSITXSADEPLOY-1661